feat: Session drilldown feature to Performance & Errors charts - #539
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds chart datapoint click-to-open sessions: frontend click handlers and time-window computation, SessionsDrawer multi-mode fetching (standard/goal/error) with sessionEvent forwarding, backend controller/service query branching for event types, API/proxy updates, and docs. ChangesChart Data Point Exploration with Session Filtering
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/app/pages/Project/tabs/Traffic/SessionsDrawer.tsx`:
- Around line 71-95: mapErrorSession is coercing ErrorAffectedSession into a
full SessionType by inserting synthetic zeros/defaults which produces misleading
UI; instead stop fabricating missing data: change mapErrorSession to return the
original ErrorAffectedSession shape (or a SessionType variant with the
truly-missing fields set to null/undefined) and update getResultSessions to
return the sessions as their real shape (e.g., union type SessionType |
ErrorAffectedSession) so the UI can render an error-specific row or display
"unknown" for missing fields rather than showing zero/default values; reference
mapErrorSession and getResultSessions when making these changes.
- Around line 193-207: The getErrorSessions branch calls fetchSessionsPage with
'getErrorSessions' but does not pass the active dashboard filters, causing
drilled sessions to ignore filtering; update the call in SessionsDrawer (the
branch using fetchSessionsPage('getErrorSessions', ...)) to include the current
filter payload (the same filter object used for the generic sessions query) so
the error-specific endpoint receives the same { period, from, to, timezone,
take, skip, ...filters, errorId } — alternatively, if the error endpoint cannot
accept filters, change the branch to call the generic sessions query (e.g.,
fetchSessionsPage('getSessions', ...)) when any dashboard filters are active so
drilled sessions respect the activeFilters.
In `@web/app/pages/Project/View/utils/chartPoint.ts`:
- Around line 48-49: The guard using eventRectsGroup.__clickAttached prevents
reattaching listeners and causes closures to capture stale
columns/onDataPointClick; replace the boolean flag with a versioned marker tied
to the current handler/data (for example store eventRectsGroup.__clickHandlerId
= `${columnsHash}:${onDataPointClickId}`) and only skip attaching when the
stored id matches the current one, or always remove existing listeners before
reattaching; update the logic in the block that sets
eventRectsGroup.__clickAttached (and the similar logic in lines 51-97) to use
this versioned marker or explicit removal so clicks use the latest columns and
onDataPointClick closures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c6a95463-b45e-4968-b2b2-bfc1b2a5b738
📒 Files selected for processing (25)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/dto/get-sessions.dto.tsbackend/apps/cloud/src/goal/goal.controller.tsbackend/apps/cloud/src/tools/tools.controller.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/dto/get-sessions.dto.tsbackend/apps/community/src/goal/goal.controller.tsdocs/content/docs/analytics-dashboard/error-tracking.mdxdocs/content/docs/analytics-dashboard/goals.mdxdocs/content/docs/analytics-dashboard/performance.mdxdocs/content/docs/api/stats.mdxweb/app/api/api.server.tsweb/app/pages/Project/View/ViewProject.helpers.tsxweb/app/pages/Project/View/utils/chartPoint.tsweb/app/pages/Project/tabs/Errors/ErrorChart.tsxweb/app/pages/Project/tabs/Errors/ErrorsView.tsxweb/app/pages/Project/tabs/Goals/GoalsView.tsxweb/app/pages/Project/tabs/Performance/PerformanceChart.tsxweb/app/pages/Project/tabs/Performance/PerformanceView.tsxweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/pages/Project/tabs/Traffic/TrafficChart.tsxweb/app/pages/Project/tabs/Traffic/TrafficView.tsxweb/app/routes/api.analytics.ts
💤 Files with no reviewable changes (1)
- backend/apps/cloud/src/tools/tools.controller.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/apps/cloud/src/analytics/analytics.service.ts`:
- Around line 6481-6489: The params object spreads filtersParams after explicit
keys which allows filter keys to inadvertently override critical query params;
change the construction of the params object so that ...filtersParams is spread
first and then explicit keys (pid, eid, groupFrom, groupTo, take, skip, ...) are
set afterwards (e.g., use { ...filtersParams, pid, eid, groupFrom, groupTo,
take, skip }) to ensure the explicit values in the params constant take
precedence over any entries produced by getFiltersQuery().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfebb664-25ee-4b76-a7b8-29961d71b348
📒 Files selected for processing (10)
backend/apps/cloud/src/analytics/analytics.controller.tsbackend/apps/cloud/src/analytics/analytics.service.tsbackend/apps/cloud/src/analytics/dto/get-error.dto.tsbackend/apps/community/src/analytics/analytics.controller.tsbackend/apps/community/src/analytics/analytics.service.tsbackend/apps/community/src/analytics/dto/get-error.dto.tsweb/app/api/api.server.tsweb/app/pages/Project/View/utils/chartPoint.tsweb/app/pages/Project/tabs/Traffic/SessionsDrawer.tsxweb/app/routes/api.analytics.ts
Changes
If applicable, please describe what changes were made in this pull request.
Community Edition support
Database migrations
Documentation
Summary by CodeRabbit
New Features
Documentation